[SYCL] Switch build of SYCL runtime to C++17 #3447

vladimirlaz · 2021-03-30T18:45:58Z

switch build of SYCL RT and SYCL unit tests to use C++17 standard;
fix build issues exposed by build with VS 2017 compiler;
workaround problem in googletest (Google Test fails to compile if a standard library implements LWG 2221 google/googletest#1616);
update documentation accordingly.

vladimirlaz · 2021-03-30T18:47:16Z

/summary:run

pvchupin · 2021-03-31T21:53:48Z

In general it's good direction. We should carefully understand impact before merge (and fix associated issues).

pvchupin

Just gating it from accidental merge.

v-klochkov · 2021-04-02T02:49:01Z

Interesting..., 9bc9b258b9d166a08426d734f428968a878d336a could be built successfully on Windows locally, but failed in CI.
I used 'python buildbot/configure.py; python buildbot/compile.py'. It looks like CI builds differently.

Or it passes locally because of different version of MSVC. I have this:
"Microsoft (R) C/C++ Optimizing Compiler Version 19.28.29336 for x64"

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

v-klochkov · 2021-04-02T04:46:31Z

Or it passes locally because of different version of MSVC. I have this:
"Microsoft (R) C/C++ Optimizing Compiler Version 19.28.29336 for x64"

@vladimirlaz
See the example (extract from our sources). https://godbolt.org/z/cc8cqr4ET
It passes with MSVC versions starting from 19.27, and fails with 19.26 and older.

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

vladimirlaz · 2021-04-02T07:25:07Z

/summary:run

pvchupin · 2021-04-05T16:00:17Z

@vladimirlaz, please update https://github.com/intel/llvm/blob/sycl/sycl/doc/GetStartedGuide.md#c-standard along with this change

vladimirlaz · 2021-04-05T18:57:11Z

@vladimirlaz, please update https://github.com/intel/llvm/blob/sycl/sycl/doc/GetStartedGuide.md#c-standard along with this change

done

bader · 2021-04-05T19:34:56Z

sycl/doc/GetStartedGuide.md

@@ -639,7 +639,7 @@ which contains all the symbols required.

 ## C++ standard

-* DPC++ runtime and headers require C++14 at least.
+* DPC++ runtime and headers require C++17 at least.


@vladimirlaz, could you confirm that there are no runtime library ABI breaking changes requiring us to increment the library version, please?

Idea for a separate PR: It would be good to have a static_assert somewhere in SYCL headers that would check that C++ is 17 or higher.
It would give user-friendly message instead of some unexpected compilation errors due to various reasons (using 'if constexpr', auto in template args, inline vars, etc. with C++14 option)

kbsmith-intel · 2021-04-05T23:01:30Z

I am very concerned about how changing over to C++ 17 requirement might adversely impact users. Seems like this is an API breaking change, and would require rolling major version of sycl headers and libraries.

pvchupin

Holding off for more testing.

kbobrovs

We should also clean-up code redundant with C++17, like this

llvm/sycl/include/CL/sycl/detail/stl_type_traits.hpp

Line 20 in 9da3104

// Type traits identical to those in std in newer versions. Can be removed when

Is this going to be a different PR?

vladimirlaz · 2021-04-09T18:21:24Z

We should also clean-up code redundant with C++17, like this

llvm/sycl/include/CL/sycl/detail/stl_type_traits.hpp

Line 20 in 9da3104

// Type traits identical to those in std in newer versions. Can be removed when

Is this going to be a different PR?

Yes this will be done in separate PR

Issue: google/googletest#1616

bader · 2021-04-15T07:11:18Z

@pvchupin, ping.

pvchupin · 2021-04-16T02:22:41Z

@vladimirlaz, please add a description to PR what this patch is doing.

vladimirlaz · 2021-04-16T05:43:12Z

@vladimirlaz, please add a description to PR what this patch is doing.

done

[SYCL] Uplift SYCL runtime standard to C++17

556b15d

bader requested a review from pvchupin March 31, 2021 06:48

Fix Windows build

13e0333

pvchupin suggested changes Mar 31, 2021

View reviewed changes

Yet another attempt to fix build on Windows

da50eb6

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

v-klochkov force-pushed the sycl branch from 9bc9b25 to da50eb6 Compare April 2, 2021 04:33

Fix for an error caused by initialization of buffer_location

50982f4

Signed-off-by: Vyacheslav N Klochkov <[email protected]>

pvchupin mentioned this pull request Apr 2, 2021

[SYCL][Doc] Update C++ standard support in GSG #3478

Merged

pvchupin mentioned this pull request Apr 5, 2021

Fails to build with C++14 #3490

Closed

pvchupin marked this pull request as ready for review April 5, 2021 16:07

pvchupin requested a review from a team as a code owner April 5, 2021 16:07

pvchupin requested review from sergey-semenov, pvchupin, bader, erichkeane, kbsmith-intel, romanovvlad, kbobrovs and smaslov-intel April 5, 2021 16:07

vladimirlaz added 2 commits April 5, 2021 21:54

Update document

f2c4ed2

Merge branch 'sycl' into sycl

69acfdf

bader reviewed Apr 5, 2021

View reviewed changes

pvchupin suggested changes Apr 5, 2021

View reviewed changes

kbobrovs reviewed Apr 6, 2021

View reviewed changes

cperkinsintel mentioned this pull request Apr 7, 2021

[WIP] [DO NOT MERGE] add std::span support #3509

Closed

Use C++17 for building SYCLUnit tests

b9f1fa4

Workaround problem in googletest

ea2cd1d

Issue: google/googletest#1616

vladimirlaz requested review from pvchupin, v-klochkov, bader and kbobrovs April 14, 2021 17:25

v-klochkov approved these changes Apr 14, 2021

View reviewed changes

bader marked this pull request as draft April 15, 2021 16:05

pvchupin marked this pull request as ready for review April 16, 2021 02:20

pvchupin approved these changes Apr 16, 2021

View reviewed changes

vladimirlaz changed the title ~~[SYCL] Uplift SYCL runtime standard to C++17~~ [SYCL] Switch build of SYCL runtime to C++17 Apr 16, 2021

pvchupin merged commit 63998b1 into intel:sycl Apr 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYCL] Switch build of SYCL runtime to C++17 #3447

[SYCL] Switch build of SYCL runtime to C++17 #3447

vladimirlaz commented Mar 30, 2021 •

edited

Loading

vladimirlaz commented Mar 30, 2021

pvchupin commented Mar 31, 2021

pvchupin left a comment

v-klochkov commented Apr 2, 2021 •

edited

Loading

v-klochkov commented Apr 2, 2021

vladimirlaz commented Apr 2, 2021

pvchupin commented Apr 5, 2021

vladimirlaz commented Apr 5, 2021

bader Apr 5, 2021

v-klochkov Apr 5, 2021 •

edited

Loading

kbsmith-intel commented Apr 5, 2021

pvchupin left a comment

kbobrovs left a comment

vladimirlaz commented Apr 9, 2021

bader commented Apr 15, 2021

pvchupin commented Apr 16, 2021

vladimirlaz commented Apr 16, 2021

[SYCL] Switch build of SYCL runtime to C++17 #3447

[SYCL] Switch build of SYCL runtime to C++17 #3447

Conversation

vladimirlaz commented Mar 30, 2021 • edited Loading

vladimirlaz commented Mar 30, 2021

pvchupin commented Mar 31, 2021

pvchupin left a comment

Choose a reason for hiding this comment

v-klochkov commented Apr 2, 2021 • edited Loading

v-klochkov commented Apr 2, 2021

vladimirlaz commented Apr 2, 2021

pvchupin commented Apr 5, 2021

vladimirlaz commented Apr 5, 2021

bader Apr 5, 2021

Choose a reason for hiding this comment

v-klochkov Apr 5, 2021 • edited Loading

Choose a reason for hiding this comment

kbsmith-intel commented Apr 5, 2021

pvchupin left a comment

Choose a reason for hiding this comment

kbobrovs left a comment

Choose a reason for hiding this comment

vladimirlaz commented Apr 9, 2021

bader commented Apr 15, 2021

pvchupin commented Apr 16, 2021

vladimirlaz commented Apr 16, 2021

vladimirlaz commented Mar 30, 2021 •

edited

Loading

v-klochkov commented Apr 2, 2021 •

edited

Loading

v-klochkov Apr 5, 2021 •

edited

Loading